feat(desktop): add KLIPY GIF search to composers - #5554

🤖 Thanks for putting this together — the relay proxy is genuinely well built. The key stays server-side with a redacted Debug impl, NIP-98 verification requires the payload hash, and there's a replay guard, membership + per-pubkey quota enforcement, an upstream response-size cap, and allowlisting so KLIPY error bodies never cross the relay boundary. That whole boundary held up under both source review and a live run against the real KLIPY API.

I'm requesting changes on two blocking issues, both verified end-to-end, plus one design question I'd like resolved before merge:

1. Selecting a GIF produces a message the relay refuses to ingest — the core feature doesn't work end-to-end. A live run against this branch got 400 {"error":"invalid: imeta url must be a local /media/ path"} on both stream (kind 9) and forum (kind 45001) sends, with zero posts persisted. Details inline at desktop/src/features/gifs/api.ts.

2. The PR breaks its own smoke test: composer-tooltip-dismiss.spec.ts still locates the Insert emoji tooltip, and Desktop Smoke E2E (2) failed that assertion on the initial attempt and both retries. Details inline at ComposerEmojiPicker.tsx.

3. KLIPY's own reference integrations couple sends to their share/report endpoint, and this PR documents skipping that as a non-goal — I don't think that call is as free as the docs suggest. Inline comment on crates/buzz-relay/src/api/gifs.rs with references.

There are also a few smaller things inline (upstream client reuse, handler-level test coverage, per-relay customer_id keying, and some nits).

🤖 All feedback from the combined review is addressed at 9db36e2 — verified each finding at source, and both blockers are confirmed fixed: external GIFs now send as content-only markdown (no imeta tag, so the relay validator is never engaged) and the tooltip smoke spec is green. The new /gifs/share endpoint sits behind the same NIP-98/membership/replay boundary as search, and live E2E against the real KLIPY provider confirmed sends persist and render on both kind 9 and forum posts, plus fully graceful degradation (no GIF tab, no errors, zero /gifs/* requests) when the relay has no KLIPY key. CI is fully green at head.

Verdict: REQUEST CHANGES

Reviewed: aea0ef8df9fc24d9aa8bf5c761ab2910026a601b..9db36e28afbe76aa3575b0665baa2b316de9d90d (exact head 9db36e28afbe76aa3575b0665baa2b316de9d90d)

Risk: high — this adds a credentialed relay-to-provider HTTP boundary plus externally hosted animated media in shared message/forum composer paths.

Behavior/contracts traced: relay config and NIP-11 capability advertisement; NIP-98/membership/replay/rate-limit admission; KLIPY search/share URL construction and response limits; renderer capability discovery and provider requests; GIF normalization/selection; content-only attachment serialization; shared composer preview/lightbox; forum/message composer reuse; loading/error/empty, motion, and accessibility paths.

Blocking findings

1. [P1] Provider redirects create an SSRF primitive and can disclose the path-embedded API key. crates/buzz-relay/src/state.rs:858-861 builds the shared reqwest client without a redirect policy. Reqwest follows redirects by default and enables Referer; meanwhile crates/buzz-relay/src/api/gifs.rs:75-105,281-282,317-321 places BUZZ_KLIPY_API_KEY in the request path for both GET search and POST share. A KLIPY 3xx can therefore make the relay connect to an arbitrary redirect target, including loopback/private/link-local services. A cross-origin Referer can carry the prior full key-bearing URL, and 307/308 can replay the share POST body.

   Author action: configure the dedicated GIF client with reqwest::redirect::Policy::none() and treat 3xx as provider failure. Add a causal regression with a redirecting fake upstream and a second listener: assert the second listener receives zero requests and the client-facing error contains neither the API key nor redirect target. Mutation-check the test by restoring default redirects and requiring it to fail.

   Verification owner: author for fix/regression; reviewer for mutation check.

2. [P1] Reduced-motion users still receive an autoplaying grid of provider animations. desktop/src/features/gifs/ui/KlipyGifPicker.tsx:108-126 renders every preview URL as an ordinary image. The only reduced-motion branch is for the 180 ms tab indicator (desktop/src/features/messages/ui/ComposerEmojiPicker.tsx:44,128-153); there is no static poster, paused state, activation gate, or stop/hide control for the feature's dominant repeating motion. One search can therefore start many indefinite animations simultaneously despite the PR's stated reduced-motion behavior.

   Author action: under prefers-reduced-motion: reduce, render a non-animated poster when the provider supplies one, or a static named placeholder requiring explicit activation before loading animated media. Add deterministic Playwright coverage using page.emulateMedia({ reducedMotion: "reduce" }) proving animated URLs are not loaded/rendered before activation, and mutation-prove the regression.

   Verification owner: author for code/regression; reviewer for exact-head browser/AX inspection.

3. [P2] GIF selection discards its accessible name in the composer preview and lightbox. The picker has the useful title (KlipyGifPicker.tsx:110-125), but klipyGifAttachment() does not preserve it and sets an empty hash (desktop/src/features/gifs/api.ts:160-169). Shared attachment UI derives all image names from that hash (desktop/src/features/messages/ui/ComposerAttachments.tsx:267,364-368,393-399,427-434), producing indistinguishable Attachment / Attachment preview names for every selected KLIPY GIF.

   Author action: preserve the normalized GIF title on the composer attachment (the existing displayLabel field is suitable), and have the shared attachment preview/lightbox use a non-empty descriptive label while hiding decorative duplicates. Add coverage selecting at least two differently titled GIFs and asserting distinct accessible names in both preview and lightbox.

   Verification owner: author for regression; reviewer for AX-tree/browser verification.

Validation

• PASS — full Desktop package tests: pnpm test, 5,312 passed / 0 failed, exact head and clean tree rechecked.
• PASS — git diff --check aea0ef8df9fc24d9aa8bf5c761ab2910026a601b..HEAD.
• PASS — all required exact-head GitHub checks are green, including Unit Tests, Desktop Core, Desktop smoke/integration, relay integration/E2E, security, cross-compile, and macOS build.
• ENVIRONMENT FAILURE — cargo test -p buzz-relay: 894 passed, 8 failed, 47 ignored; all eight failures were database-unavailable shapes (PoolTimedOut or dependent status mismatches). Exact-head Unit Tests and relay integration/E2E checks are green, so this is not attributed to the PR.
• REVIEWER-LOCAL CONFIDENCE GAP — a focused smoke row was 1 pass / 1 fail twice while reusing a port-4173 server; the positive capability row did not observe mocked /info. This conflicts with green exact-head CI and is not classified as another author defect.

Manual/native evidence: no new exact-head native Desktop/AX journey was completed in this review. Source inspection establishes the three defects above independently.

Residual risk: configured-route auth/replay/membership/quota rejection is not directly covered at the handler boundary; source composes the established shared gates correctly. Native keyboard/AX, dark theme, narrow-window, and live forum-send journeys remain unwitnessed. These are confidence gaps, not additional author actions.

— :bot: Jude’s code review agent